From 078b6707f793534a1e37960b6b383e5876dfb27c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 21 Feb 2016 20:58:08 -0800 Subject: [PATCH] Move Context::get_package to returning a Result The propagate all the `try!`-s needed upwards --- src/cargo/ops/cargo_rustc/context.rs | 63 +++++++++++++---------- src/cargo/ops/cargo_rustc/custom_build.rs | 18 ++++--- src/cargo/ops/cargo_rustc/fingerprint.rs | 3 +- src/cargo/ops/cargo_rustc/job_queue.rs | 24 +++++---- src/cargo/ops/cargo_rustc/mod.rs | 10 ++-- 5 files changed, 67 insertions(+), 51 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 3b8fd290b..8d88ac384 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -338,7 +338,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// For a package, return all targets which are registered as dependencies /// for that package. - pub fn dep_targets(&self, unit: &Unit<'a>) -> Vec> { + pub fn dep_targets(&self, unit: &Unit<'a>) -> CargoResult>> { if unit.profile.run_custom_build { return self.dep_run_custom_build(unit) } else if unit.profile.doc { @@ -347,7 +347,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { let id = unit.pkg.package_id(); let deps = self.resolve.deps(id).into_iter().flat_map(|a| a); - let mut ret = deps.filter(|dep| { + let mut ret = try!(deps.filter(|dep| { unit.pkg.dependencies().iter().filter(|d| { d.name() == dep.name() }).any(|d| { @@ -385,22 +385,26 @@ impl<'a, 'cfg> Context<'a, 'cfg> { true }) }).filter_map(|id| { - let pkg = self.get_package(id); - pkg.targets().iter().find(|t| t.is_lib()).map(|t| { - Unit { - pkg: pkg, - target: t, - profile: self.lib_profile(id), - kind: unit.kind.for_target(t), + match self.get_package(id) { + Ok(pkg) => { + pkg.targets().iter().find(|t| t.is_lib()).map(|t| { + Ok(Unit { + pkg: pkg, + target: t, + profile: self.lib_profile(id), + kind: unit.kind.for_target(t), + }) + }) } - }) - }).collect::>(); + Err(e) => Some(Err(e)) + } + }).collect::>>()); // If this target is a build script, then what we've collected so far is // all we need. If this isn't a build script, then it depends on the // build script if there is one. if unit.target.is_custom_build() { - return ret + return Ok(ret) } ret.extend(self.dep_build_script(unit)); @@ -409,7 +413,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { // didn't include `pkg` in the return values, so we need to special case // it here and see if we need to push `(pkg, pkg_lib_target)`. if unit.target.is_lib() { - return ret + return Ok(ret) } ret.extend(self.maybe_lib(unit)); @@ -425,20 +429,21 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } })); } - ret + Ok(ret) } /// Returns the dependencies needed to run a build script. /// /// The `unit` provided must represent an execution of a build script, and /// the returned set of units must all be run before `unit` is run. - pub fn dep_run_custom_build(&self, unit: &Unit<'a>) -> Vec> { + pub fn dep_run_custom_build(&self, unit: &Unit<'a>) + -> CargoResult>> { // If this build script's execution has been overridden then we don't // actually depend on anything, we've reached the end of the dependency // chain as we've got all the info we're gonna get. let key = (unit.pkg.package_id().clone(), unit.kind); if self.build_state.outputs.lock().unwrap().contains_key(&key) { - return Vec::new() + return Ok(Vec::new()) } // When not overridden, then the dependencies to run a build script are: @@ -454,7 +459,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> { profile: &self.profiles.dev, ..*unit }; - self.dep_targets(&tmp).iter().filter_map(|unit| { + let deps = try!(self.dep_targets(&tmp)); + Ok(deps.iter().filter_map(|unit| { if !unit.target.linkable() || unit.pkg.manifest().links().is_none() { return None } @@ -463,11 +469,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> { profile: self.build_script_profile(unit.pkg.package_id()), kind: Kind::Host, // build scripts always compiled for the host ..*unit - })).collect() + })).collect()) } /// Returns the dependencies necessary to document a package - fn doc_deps(&self, unit: &Unit<'a>) -> Vec> { + fn doc_deps(&self, unit: &Unit<'a>) -> CargoResult>> { let deps = self.resolve.deps(unit.pkg.package_id()).into_iter(); let deps = deps.flat_map(|a| a).filter(|dep| { unit.pkg.dependencies().iter().filter(|d| { @@ -479,16 +485,20 @@ impl<'a, 'cfg> Context<'a, 'cfg> { _ => false, } }) - }).filter_map(|dep| { - let dep = self.get_package(dep); - dep.targets().iter().find(|t| t.is_lib()).map(|t| (dep, t)) + }).map(|dep| { + self.get_package(dep) }); // To document a library, we depend on dependencies actually being // built. If we're documenting *all* libraries, then we also depend on // the documentation of the library being built. let mut ret = Vec::new(); - for (dep, lib) in deps { + for dep in deps { + let dep = try!(dep); + let lib = match dep.targets().iter().find(|t| t.is_lib()) { + Some(lib) => lib, + None => continue, + }; ret.push(Unit { pkg: dep, target: lib, @@ -512,7 +522,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { if unit.target.is_bin() { ret.extend(self.maybe_lib(unit)); } - ret + Ok(ret) } /// If a build script is scheduled to be run for the package specified by @@ -559,9 +569,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } /// Gets a package for the given package id. - pub fn get_package(&self, id: &PackageId) -> &'a Package { - // TODO: remove this unwrap() -- happens in a later commit - self.packages.get(id).unwrap() + pub fn get_package(&self, id: &PackageId) -> CargoResult<&'a Package> { + self.packages.get(id) } /// Get the user-specified linker for a particular host or target diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index e52a12c40..929e54b93 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -124,7 +124,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // This information will be used at build-time later on to figure out which // sorts of variables need to be discovered at that time. let lib_deps = { - cx.dep_run_custom_build(unit).iter().filter_map(|unit| { + try!(cx.dep_run_custom_build(unit)).iter().filter_map(|unit| { if unit.profile.run_custom_build { Some((unit.pkg.manifest().links().unwrap().to_string(), unit.pkg.package_id().clone())) @@ -372,25 +372,27 @@ impl BuildOutput { /// The given set of targets to this function is the initial set of /// targets/profiles which are being built. pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, - units: &[Unit<'b>]) { + units: &[Unit<'b>]) + -> CargoResult<()> { let mut ret = HashMap::new(); for unit in units { - build(&mut ret, cx, unit); + try!(build(&mut ret, cx, unit)); } cx.build_scripts.extend(ret.into_iter().map(|(k, v)| { (k, Arc::new(v)) })); + return Ok(()); // Recursive function to build up the map we're constructing. This function // memoizes all of its return values as it goes along. fn build<'a, 'b, 'cfg>(out: &'a mut HashMap, BuildScripts>, cx: &Context<'b, 'cfg>, unit: &Unit<'b>) - -> &'a BuildScripts { + -> CargoResult<&'a BuildScripts> { // Do a quick pre-flight check to see if we've already calculated the // set of dependencies. if out.contains_key(unit) { - return &out[unit] + return Ok(&out[unit]) } let mut ret = BuildScripts::default(); @@ -398,8 +400,8 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, if !unit.target.is_custom_build() && unit.pkg.has_custom_build() { add_to_link(&mut ret, unit.pkg.package_id(), unit.kind); } - for unit in cx.dep_targets(unit).iter() { - let dep_scripts = build(out, cx, unit); + for unit in try!(cx.dep_targets(unit)).iter() { + let dep_scripts = try!(build(out, cx, unit)); if unit.target.for_host() { ret.plugins.extend(dep_scripts.to_link.iter() @@ -416,7 +418,7 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, add_to_link(prev, &pkg, kind); } prev.plugins.extend(ret.plugins); - prev + Ok(prev) } // When adding an entry to 'to_link' we only actually push it on if the diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 386ed7792..d0007f52d 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -315,7 +315,8 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // elsewhere. Also skip fingerprints of binaries because they don't actually // induce a recompile, they're just dependencies in the sense that they need // to be built. - let deps = try!(cx.dep_targets(unit).iter().filter(|u| { + let deps = try!(cx.dep_targets(unit)); + let deps = try!(deps.iter().filter(|u| { !u.target.is_custom_build() && !u.target.is_bin() }).map(|unit| { calculate(cx, unit).map(|fingerprint| { diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index 6ef8f7d35..ff35daac2 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -68,14 +68,16 @@ impl<'a> JobQueue<'a> { } } - pub fn enqueue<'cfg>(&mut self, cx: &Context<'a, 'cfg>, - unit: &Unit<'a>, job: Job, fresh: Freshness) { + pub fn enqueue<'cfg>(&mut self, + cx: &Context<'a, 'cfg>, + unit: &Unit<'a>, + job: Job, + fresh: Freshness) -> CargoResult<()> { let key = Key::new(unit); - self.queue.queue(Fresh, - key, - Vec::new(), - &key.dependencies(cx)).push((job, fresh)); + let deps = try!(key.dependencies(cx)); + self.queue.queue(Fresh, key, Vec::new(), &deps).push((job, fresh)); *self.counts.entry(key.pkg).or_insert(0) += 1; + Ok(()) } /// Execute all jobs necessary to build the dependency graph. @@ -243,14 +245,16 @@ impl<'a> Key<'a> { } } - fn dependencies<'cfg>(&self, cx: &Context<'a, 'cfg>) -> Vec> { + fn dependencies<'cfg>(&self, cx: &Context<'a, 'cfg>) + -> CargoResult>> { let unit = Unit { - pkg: cx.get_package(self.pkg), + pkg: try!(cx.get_package(self.pkg)), target: self.target, profile: self.profile, kind: self.kind, }; - cx.dep_targets(&unit).iter().filter_map(|unit| { + let targets = try!(cx.dep_targets(&unit)); + Ok(targets.iter().filter_map(|unit| { // Binaries aren't actually needed to *compile* tests, just to run // them, so we don't include this dependency edge in the job graph. if self.target.is_test() && unit.target.is_bin() { @@ -258,7 +262,7 @@ impl<'a> Key<'a> { } else { Some(Key::new(unit)) } - }).collect() + }).collect()) } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 0306c29b3..b71da0fcc 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -92,7 +92,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(pkg_targets: &'a PackagesToBuild<'a>, let mut queue = JobQueue::new(&cx); try!(cx.prepare(root)); - custom_build::build_map(&mut cx, &units); + try!(custom_build::build_map(&mut cx, &units)); for unit in units.iter() { // Build up a list of pending jobs, each of which represent @@ -129,7 +129,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(pkg_targets: &'a PackagesToBuild<'a>, if !unit.target.is_lib() { continue } // Include immediate lib deps as well - for unit in cx.dep_targets(unit).iter() { + for unit in try!(cx.dep_targets(unit)).iter() { let pkgid = unit.pkg.package_id(); if !unit.target.is_lib() { continue } if unit.profile.doc { continue } @@ -191,11 +191,11 @@ fn compile<'a, 'cfg: 'a>(cx: &mut Context<'a, 'cfg>, let dirty = work.then(dirty); (dirty, fresh, freshness) }; - jobs.enqueue(cx, unit, Job::new(dirty, fresh), freshness); + try!(jobs.enqueue(cx, unit, Job::new(dirty, fresh), freshness)); drop(p); // Be sure to compile all dependencies of this target as well. - for unit in cx.dep_targets(unit).iter() { + for unit in try!(cx.dep_targets(unit)).iter() { try!(compile(cx, jobs, unit)); } Ok(()) @@ -562,7 +562,7 @@ fn build_deps_args(cmd: &mut CommandPrototype, cx: &Context, unit: &Unit) cmd.env("OUT_DIR", &layout.build_out(unit.pkg)); } - for unit in cx.dep_targets(unit).iter() { + for unit in try!(cx.dep_targets(unit)).iter() { if unit.target.linkable() { try!(link_to(cmd, cx, unit)); } -- 2.30.2